Repository navigation
fix(cli): resolve API versions consistently for generic resource operations - #13232
lakshmimsft wants to merge 1 commit into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
🟡 Changes recommended
Empty-version handling and coverage for default and multi-version selection branches need correction.
1 open finding
What changed in this PR
Fixes generic resource creation to resolve provider API versions consistently and deterministically.
Changes:
- Resolves API versions during resource creation.
- Prefers provider defaults and stable fallback versions.
- Adds wire-level tests and a CLI example.
| File | Summary |
|---|---|
pkg/cli/cmd/resource/create/create.go |
Documents creating arbitrary registered resource types. |
pkg/cli/clients/management.go |
Updates API-version resolution and generic client construction. |
pkg/cli/clients/management_test.go |
Adds API-version request coverage and updates test setup. |
🧠 Review effort: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Unit Tests 2 files ±0 460 suites ±0 15m 34s ⏱️ -1s Results for commit 4e632b4. ± Comparison against base commit ebe182b. This pull request removes 4 and adds 8 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13232 +/- ##
=======================================
Coverage 60.66% 60.66%
=======================================
Files 777 777
Lines 45875 45878 +3
=======================================
+ Hits 27829 27832 +3
Misses 18046 18046 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
f0228f9 to
d13e10a
Compare
…ations Signed-off-by: lakshmimsft <ljavadekar@microsoft.com>
d13e10a to
4e632b4
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |

Summary
rad resource createsentapi-version=2023-10-01-previewon every request, regardless of the resource type being created, and regardless of any API version that type declared. This PR makes it resolve the API version from the resource provider like the other generic resource operations already do, and makes that resolution deterministic for all of them.Three related changes:
CreateOrUpdateResourcenow resolves its API version. It was the only generic CRUD operation that did not. It called a separate helper,createGenericClient, whose version argument was variadic — and the sole caller passed nothing, soclientOptions.APIVersionwas never set and the generated client's hardcoded2023-10-01-previewapplied. The resource provider's advertised API versions were not overridden on this path; they were never looked up at all, becausegetApiVersionsForResourceTypewas never called from it. Declaring an API version on a resource type therefore had no effect onrad resource create.createGenericClientalso lacked theisRadiusCoreTypepin thatgetGenericClientapplies, so evenRadius.Coretypes were created as2023-10-01-preview.createGenericClientis dead once that call site moves togetGenericClient, so it is deleted.This went unnoticed because
Applications.Coregenuinely is2023-10-01-preview, so the hardcoded default was accidentally correct for the only types being created today. EveryRadius.*type is served at2025-08-01-previewonly, so creates against them were addressed with a version their provider does not serve. Fixing this is a prerequisite for #11803.API version selection is now deterministic. The resolver returned
[]stringbuilt frommaps.Keys(...), and both consumers used onlyapiVersions[0]. Because Go randomizes map iteration order, a resource type advertising more than one version could get a different API version on each invocation. The resolver now returns a singlestring: it prefers the resource provider's declareddefaultApiVersion, and otherwise sorts and takes the lowest. The signature change ([]stringtostring) reflects what the function always did — a single HTTP request carries a singleapi-version, so the caller could never use more than one element.Behavior when a type advertises no API versions is unchanged. The resolver returns an empty version, which leaves the generated client on its built-in
2023-10-01-previewdefault, exactly as before. This case is deliberately left alone rather than turned into an error — see "Reason for change" below.This is a no-op for every resource type that exists today: no manifest in
deploy/manifest/setsdefaultApiVersion, and every built-in type advertises exactly one API version (2025-08-01-preview), so the default-preference branch never fires and sorting a one-element list returns that element. The change can only select differently than before for a user-defined type that declares a default or advertises two or more versions.Reason for change
Groundwork for #11803, which makes the new
Radius.*resource types the default for the imperative CLI commands. Those commands cannot become the default whilerad resource createaddresses every type with theApplications.CoreAPI version. No separate issue was filed for this bug; it surfaced while preparing that work.How to test
Automated coverage is included.
Test_CreateOrUpdateResource_APIVersionasserts the actualapi-versionquery parameter on the outgoingPUTusing a real transport, rather than a mock client factory — the factory short-circuitsgetGenericClientbefore client options are applied, so a factory-based test cannot observe the wire format. The four cases cover a provider-advertised version, a non-Radius.Coretype resolving dynamically, theRadius.Corepin, and a lookup failure asserting that noPUTis sent.Each case was confirmed to fail with the production fix reverted, emitting
api-version=2023-10-01-preview.Commands run, with results:
go build ./...go vet ./...go test ./pkg/cli/... -count=1go test ./pkg/cli/clients/ -race -count=1gofmt -lon changed filesFunctional and end-to-end suites under
test/compile (covered bygo vet ./...) but were not executed, as they require a live cluster. They exercise only built-in resource types, for which this change is provably a no-op per the Summary.To verify manually against a cluster, create a resource of a non-
Applications.Coretype and confirm the request carries the provider's API version rather than2023-10-01-preview:rad resource create 'Radius.Compute/containers' mycontainer -f /path/to/input.jsonFile change summary
pkg/cli/clients/management.goCreateOrUpdateResourceresolves the API version before building its client. Deleted the now-unusedcreateGenericClient. RenamedgetApiVersionsForResourceTypetogetAPIVersionForResourceType, returning a single version that prefersdefaultApiVersionand otherwise sorts for stability. ChangedgetGenericClientto take a singleapiVersion stringand updated all six call sites. TheRadius.Corepin is retained.pkg/cli/clients/management_test.goTest_CreateOrUpdateResource_APIVersion, a four-case table test asserting the outgoingapi-versionquery parameter via a real transport. Updated the existingCreateOrUpdateResourcesubtest to wireresourceProviderClientFactory, which the new lookup requires.pkg/cli/cmd/resource/create/create.goRadius.Compute/containersexample to the command help, showing that any registered resource type is supported.